Skip to content

Comments

upgrade redb file format to v3#7917

Open
owanikin wants to merge 27 commits intosigp:unstablefrom
owanikin:upgrade-redb-v3
Open

upgrade redb file format to v3#7917
owanikin wants to merge 27 commits intosigp:unstablefrom
owanikin:upgrade-redb-v3

Conversation

@owanikin
Copy link

@owanikin owanikin commented Aug 21, 2025

Issue Addressed

#7911

Proposed Changes

  • Add migration v29 for Redb file-format upgrade.
  • Introduce migration_schema_v29.rs with:
    • Upgrade path gated under the redb feature.
    • Explicit irreversible downgrade (returns error).
  • Register migration in schema_change.rs.

eserilev
eserilev reviewed Aug 21, 2025
View reviewed changes
beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs Outdated
pub fn downgrade_from_v29<T: BeaconChainTypes>(
_d: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
) -> Result<Vec<KeyValueStoreOp>, Error> {
Err(Error::MigrationError("Cannot downgrade from v29: Redb file format upgrade is irreversible".to_string()))
Copy link
Member

@eserilev eserilev Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the downgrade should only fail if redb is enabled and the database backend is redb

In all other cases it should just be a no-op

@eserilev
Copy link
Member

eserilev commented Aug 21, 2025

Just wanted to document here that redb v2.6.2 creates the db using the v3 format by default

https://docs.rs/redb/2.6.2/src/redb/db.rs.html#1202-1220

So once we push this change, new redb databases will be using the correct v3 format as well

@eserilev eserilev marked this pull request as ready for review September 8, 2025 21:44
eserilev
eserilev requested changes Sep 8, 2025
View reviewed changes
Copy link
Member

@eserilev eserilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sticking through with this! This looks really good, mostly just have some things to remove

beacon_node/beacon_chain/src/schema_change/migration_schema_v29.rs Outdated
pub fn upgrade_to_v29<T: BeaconChainTypes>(
db: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
) -> Result<Vec<KeyValueStoreOp>, Error> {
if db.is_redb() {
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was nice for testing, but we no longer need this, you can just call upgrade

beacon_node/store/src/database/interface.rs Outdated
}
#[cfg(feature = "redb")]
BeaconNodeBackend::Redb(redb) => {
info!("Running Redb v29 migration");
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update to this log to "Updating Redb file format to v3"

beacon_node/store/src/database/interface.rs Outdated
match self {
#[cfg(feature = "leveldb")]
BeaconNodeBackend::LevelDb(_) => {
info!("LevelDB upgrade: no migration needed");
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update this log to "LevelDB, no upgrade required" and downgrade this log to debug

beacon_node/store/src/hot_cold_store.rs Outdated
Comment on lines 273 to 277
#[cfg(feature = "redb")]
let _database_backend = DatabaseBackend::Redb;
#[cfg(feature = "leveldb")]
let database_backend = DatabaseBackend::LevelDb;

Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, this must be something I pushed up, we can remove this

beacon_node/store/src/hot_cold_store.rs Outdated
/// Chain spec.
pub spec: Arc<ChainSpec>,
/// Database backend type
pub database_backend: Option<DatabaseBackend>,
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this as well

beacon_node/store/src/hot_cold_store.rs Outdated
@@ -1,3 +1,5 @@
#[cfg(feature = "redb")]
use crate::config::DatabaseBackend;
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed

beacon_node/store/src/hot_cold_store.rs Outdated
config,
hierarchy,
spec,
database_backend: None,
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed

beacon_node/store/src/hot_cold_store.rs Outdated
config,
hierarchy,
spec,
database_backend: Some(database_backend),
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove as well

lighthouse/Cargo.toml Outdated

[features]
default = ["slasher-lmdb", "beacon-node-leveldb"]
default = ["slasher-lmdb", "beacon-node-redb"]
Copy link
Member

@eserilev eserilev Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set default back to leveldb (sorry I think I pushed up this change)

eserilev
eserilev reviewed Sep 9, 2025
View reviewed changes
beacon_node/store/src/database/interface.rs
#[cfg(feature = "redb")]
BeaconNodeBackend::Redb(redb) => {
info!("Updating Redb file format to v3");
redb.upgrade()?;
Copy link
Member

@eserilev eserilev Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note thta upgrade returns true is successful and false if its already in the v3 format

https://docs.rs/redb/2.6.2/redb/struct.Database.html#method.upgrade

In all other failure cases it will raise an error

eserilev
eserilev approved these changes Sep 9, 2025
View reviewed changes
Copy link
Member

@eserilev eserilev left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@michaelsproul whenever you have a chance, would be great to get your review as well. Thanks!

@eserilev eserilev added the ready-for-review The code is ready for review label Sep 9, 2025
@mergify
Copy link

mergify bot commented Sep 9, 2025

Some required checks have failed. Could you please take a look @owanikin? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 9, 2025
@eserilev
Copy link
Member

eserilev commented Sep 9, 2025

@owanikin we have a small ci failure, can you run cargo sort --workspace and push those changes up when you get the chance

@owanikin owanikin mentioned this pull request Sep 15, 2025
Open
@eserilev eserilev added the v8.1.0 Post-Fulu release label Oct 14, 2025
@eserilev eserilev added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Oct 14, 2025
@mergify
Copy link

mergify bot commented Nov 19, 2025

Some required checks have failed. Could you please take a look @owanikin? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Nov 19, 2025
@eserilev eserilev added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Dec 3, 2025
@mergify
Copy link

mergify bot commented Dec 3, 2025

Some required checks have failed. Could you please take a look @owanikin? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Dec 3, 2025
@mergify
Copy link

mergify bot commented Jan 2, 2026

Hi @owanikin, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time.

@mergify mergify bot closed this Jan 2, 2026
@mergify mergify bot added the stale Stale PRs that have been inactive and is now outdated label Jan 2, 2026
@eserilev eserilev reopened this Feb 10, 2026
@jimmygchen jimmygchen removed the v8.1.0 Post-Fulu release label Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eserilev eserilev eserilev approved these changes

@michaelsproul michaelsproul Awaiting requested review from michaelsproul michaelsproul is a code owner

Assignees

No one assigned

Labels

database stale Stale PRs that have been inactive and is now outdated waiting-on-author The reviewer has suggested changes and awaits thier implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@owanikin @eserilev @jimmygchen